Specify the network when deploying the VM from content library #2407
Conversation
eks-distro-bot
added
the
size/M
Codecov Report
@@ Coverage Diff @@
## main #2407 +/- ##
==========================================
+ Coverage 56.25% 56.54% +0.28%
==========================================
Files 304 305 +1
Lines 24689 24798 +109
==========================================
+ Hits 13889 14021 +132
+ Misses 9507 9479 -28
- Partials 1293 1298 +5
Continue to review full report at Codecov.
|
eks-distro-bot
added
size/L
abhinavmpandey08
force-pushed
the
provide-network-during-deploy
branch
from
ad1264c to
fed3ea3
Compare
abhinavmpandey08
force-pushed
the
provide-network-during-deploy
branch
from
c158605 to
7e387ab
Compare
Signed-off-by: Abhinav Pandey <abhinavmpandey08@gmail.com>
abhinavmpandey08
force-pushed
the
provide-network-during-deploy
branch
from
7e387ab to
b585a79
Compare
| @@ -269,9 +276,9 @@ func (g *Govc) CreateLibrary(ctx context.Context, datastore, library string) err | |||
| return nil | |||
| } | |||
|
|
|||
| func (g *Govc) DeployTemplateFromLibrary(ctx context.Context, templateDir, templateName, library, datacenter, datastore, resourcePool string, resizeBRDisk bool) error { | |||
| func (g *Govc) DeployTemplateFromLibrary(ctx context.Context, templateDir, templateName, library, datacenter, datastore, network, resourcePool string, resizeBRDisk bool) error { | |||
There was a problem hiding this comment.
Idk, if you have looked at my tink e2e PR yet, but I had to do very similar thing as I wanted to use this code to deploy OVA's in the lab. I tried to change as little as possible as I only needed it for the tests, but there are a ton more options in deploy options. Rather, than passing them individually why don't we just take deploy options as a param to account for all possible deploy options?
You can see the options I am using for tink e2e here: https://github.com/aws/eks-anywhere/pull/2031/files#diff-5dfd6bc5a441b6e4027710c63250668f8e0af8e594dfcb6c3c11067cca3e5845
But there are many more options available that we could support.
There was a problem hiding this comment.
Link above is code in test package, actual govc changes I made were minimal to not disrupt. You can see them here: https://github.com/aws/eks-anywhere/pull/2031/files#diff-1b2bffa403bbd82a1ba98650cb15df32447b074ad23e1cc1cf7df592eca4554d
Thoughts?
There was a problem hiding this comment.
oh just saw your comment. For CAPV provider, I think we only needed the network option. The disk provisioning option is static. But if more options are needed, then yeah we can make it more generic and provide a slice of options
There was a problem hiding this comment.
Ok, cool. I'm fine revisiting later as if needed.
There was a problem hiding this comment.
yeah seems like you already handled that in your PR. Will review it and try to get it merged tomorrow so you don't have to deal with a bunch of merge conflicts
There was a problem hiding this comment.
Yea, I have to get code build in order before merging, but please do review as I want to merge before end of week.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavmpandey08 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description of changes:
Provide a network when deploying VM from a template in the content library.
If the network is not provided, vSphere will pick some network and try to deploy the vm on that network which can have unintended consequences.
Tested and verified with both bottlerocket and ubuntu
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.